Exploring possible bug in get_complete_scenarios_for_magicc - #70
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #70 +/- ##
==========================================
+ Coverage 72.56% 72.64% +0.08%
==========================================
Files 44 44
Lines 1972 1978 +6
Branches 189 190 +1
==========================================
+ Hits 1431 1437 +6
Misses 499 499
Partials 42 42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I see and understand the bug. Thanks for spotting and fixing. I assume this didn't affect anything in SCI, because we haven't noticed until now? |
I think it was unlikely to occur in any real scenario. I only ran into it because I was playing around with some synthetic scenarios and toggled the checks off. I wanted to see the ocean's response to cumulative CO2, so I basically set everything to 0 except a constant |
|
Ok cool. Let's add a test specifically for these cases (but just run 1 MAGICC cfg or even skip running in MAGICC), then do the fix, then we can merge |
|
Should we accept to work with a duplicated scenario input (and drop duplicates) ? Or do we raise an error? |
|
If there are two trajectories which are identical for model, scenario and variable, raise an error |
|
This should fix it. |
znichollscr
left a comment
znichollscr
left a comment
There was a problem hiding this comment.
Nice. Bit of tidy up then good to merge
| .duplicated(subset=["model", "scenario", "variable"]) | ||
| .any() | ||
| ): | ||
| msg = "'scenarios' has duplicate index: model, scenario, variable" |
There was a problem hiding this comment.
Let's add the duplicate model - scenario - variable to the error message please
| @@ -0,0 +1,88 @@ | |||
| """ | |||
| Tests of `gcages.cmip7_scenariomip.scm_running.get_complete_scenarios_for_magicc | |||
There was a problem hiding this comment.
| Tests of `gcages.cmip7_scenariomip.scm_running.get_complete_scenarios_for_magicc | |
| Tests of `gcages.cmip7_scenariomip.scm_running.get_complete_scenarios_for_magicc` |
2 participants
Description
Exploring possible bug in
cmip7_scenariomip/scm_running/get_complete_scenarios_for_magicc.I discovered an unexpected behavior (to me) playing around with artificial constant emission and I though we might have some bug.
Checklist`
Please confirm that this pull request has done the following:
Documentation added (where applicable)changelog/